Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate to Unified Bridge, introduce the ICTT bridge #82

Merged
merged 15 commits into from
Dec 4, 2024

Conversation

meeh0w
Copy link
Member

@meeh0w meeh0w commented Oct 25, 2024

Description

  • 👋 Bye bye @avalabs/bridge-sdk (in the frontend -- some backend parts need to stay for a while)
  • 🤗 Embrace the @avalabs/bridge-unified (comes in with CTTP, ICTT (erc20-erc20) and Avalanche Bridge integrations)

Changes

  • Lots of code removed
    • Some parts that are still using the @avalabs/bridge-sdk are still left - during the migration phase, some people may have old BridgeTransactions either in progress or completed, but not acknowledged in the UI yet. We may need a schema migration, depending on how the UnifiedBridgeSDK handles BTC tracking.
  • Simplified data flows:
    • No more useAvalancheBridge() / useBtcBridge() / useEthBridge() / useUnifiedBridge() hooks. The old useBridge() which previously merged the capabilities of them all is now enough to handle everything.
    • No more BridgeFormETH / BridgeFormBTC / BridgeFormAVAX or BridgeFormUnified. The BridgeForm got promoted and is used directly by Bridge.tsx page.
    • Some unused components & utils got removed

The new user path

  1. User selects assets from those available for bridging on the current active network
  2. Target chains are loaded after asset is selected. The first available target chain is pre-selected.
    • User may change the target chain if there are more options than just one.
  3. User may change the source network at any point (either by changing it in the form, or via the networks widget)
    • If asset is present on the new network, the token & amount is kept in-tact.
    • If asset is not present on the new network, form is reset.

Testing

  • Entire Bridge suite must be tested.
  • Remember about testing the Core Web integration as well.
  • Test how it behaves when the related feature flags are toggled:
    • unified-bridge-cctp
    • unified-bridge-ictt
    • unified-bridge-ab-evm
    • unified-bridge-ab-ava-to-btc
    • unified-bridge-ab-btc-to-ava

Screenshots:

Feature Flags examples

When Bitcoin -> Avalanche bridge IS disabled
image

The moment when selected bridge route (Avalanche -> Bitcoin) gets disabled

Screen.Recording.2024-11-06.at.12.23.58.mov

Checklist for the author

  • I've covered new/modified business logic with Jest test cases.
  • I've tested the changes myself before sending it to code review and QA.

@meeh0w meeh0w changed the title feat: integrate ICTT bridge feat: migrate to Unified Bridge, introduce the ICTT bridge Nov 6, 2024
gergelylovas
gergelylovas previously approved these changes Nov 28, 2024
Copy link
Contributor

@bferenc bferenc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this changes, just FYI: the tokenlist endpoint responds with an empty array fro sepolia testnet, so it's only possible to use ETH for briding from sepolia to fuji C.

My sepolia -> fuji C ethereum bridge finished successfully, however the tracker just keeps trying 😅
image

And the BTC bridge transaction is not recognized in the activity log
image

Do we want to address these issues in this PR?

@meeh0w
Copy link
Member Author

meeh0w commented Dec 4, 2024

@bferenc, thanks for testing! 🍻

  • tokenlist was this way for a long long time. I agree we could populate it with some tokens to make our lives easier.
  • I tried to reproduce the hanging tracking screen, but couldn't (I tried once for CCTP and twice for ETH -> ETH.e, it worked every time)
  • I fixed the BTC bridge tx not being recognized as bridge tx (the @avalabs/bridge-unified version update takes care of that)
  • I addressed some issues with amount formatting, gas calculation (bridge amount was always taken into consideration there, even when bridged asset was not the gas token) and with some values not being reset when token was being changed (i.e. receive amount would stay populated)

@meeh0w meeh0w merged commit f7c9f43 into main Dec 4, 2024
6 checks passed
@meeh0w meeh0w deleted the feat/unified-bridging branch December 4, 2024 15:21
Charon-Fan pushed a commit to KeystoneHQ/core-extension that referenced this pull request Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants